-
Notifications
You must be signed in to change notification settings - Fork 523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port robot_state to ROS 2 #59
Conversation
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE | ||
* POSSIBILITY OF SUCH DAMAGE. | ||
*********************************************************************/ | ||
* Software License Agreement (BSD License) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert these kind of changes
@param timeout The timeout passed to the kinematics solver on each attempt | ||
@param constraint A state validity constraint to be required for IK solutions */ | ||
bool setFromIK(const JointModelGroup* group, const geometry_msgs::Pose& pose, double timeout = 0.0, | ||
const GroupStateValidityCallbackFn& constraint = GroupStateValidityCallbackFn(), | ||
bool setFromIK(const JointModelGroup* group, const geometry_msgs::msg::Pose& pose, unsigned int attempts = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attempts
parameter was removed so it shouldn't be added here again. This also accounts for the removed deprecated functions below.
const EigenSTL::vector_Isometry3d& attach_trans, const std::set<std::string>& touch_links, | ||
const std::string& link_name, | ||
const trajectory_msgs::JointTrajectory& detach_posture = trajectory_msgs::JointTrajectory()); | ||
void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert
// ******************************************** | ||
// * Internal (hidden) functions | ||
// ******************************************** | ||
|
||
namespace | ||
{ | ||
static bool _jointStateToRobotState(const sensor_msgs::JointState& joint_state, RobotState& state) | ||
// Logger | ||
rclcpp::Logger LOGGER = rclcpp::get_logger("moveit").get_child("robot_state"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rclcpp::Logger LOGGER = rclcpp::get_logger("moveit").get_child("robot_state"); | |
const rclcpp::Logger LOGGER = rclcpp::get_logger("moveit").get_child("robot_state"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, LOGGER
is used outside this anonymous namespace so it should stay at the same position as LOGNAME
before.
error = true; | ||
continue; | ||
} | ||
// if frames do not mach, attempt to transform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
@@ -1644,6 +1633,9 @@ bool RobotState::setFromIK(const JointModelGroup* jmg, const EigenSTL::vector_Is | |||
if (timeout < std::numeric_limits<double>::epsilon()) | |||
timeout = jmg->getDefaultIKTimeout(); | |||
|
|||
if (attempts == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
@@ -1652,32 +1644,66 @@ bool RobotState::setFromIK(const JointModelGroup* jmg, const EigenSTL::vector_Is | |||
// Bijection | |||
const std::vector<unsigned int>& bij = jmg->getKinematicsSolverJointBijection(); | |||
|
|||
bool first_seed = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix code sync
ros::Publisher pub_aabb = nh.advertise<visualization_msgs::Marker>("/visualization_aabb", 10); | ||
ros::Publisher pub_obb = nh.advertise<visualization_msgs::Marker>("/visualization_obb", 10); | ||
rclcpp::init(argc, argv); | ||
auto node = rclcpp::Node::make_shared("visualize_pr2"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to create a shared pointer? I think rclcpp::Node node("visualize_pr2")
would be sufficient.
ros::Publisher pub_obb = nh.advertise<visualization_msgs::Marker>("/visualization_obb", 10); | ||
rclcpp::init(argc, argv); | ||
auto node = rclcpp::Node::make_shared("visualize_pr2"); | ||
auto pub_aabb = node->create_publisher<visualization_msgs::Marker>("/visualization_aabb", rmw_qos_profile_default); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is rmw_qos_profile_default
defined?
msg.type = visualization_msgs::Marker::CUBE; | ||
msg.color.a = 0.5; | ||
msg.lifetime.sec = 3000; | ||
auto msg = std::make_shared<visualization_msgs::Marker>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's perfectly fine to use a reference instead of a shared pointer here.
@ahcorde I see that you attempted to revert some of the unwanted changes. Unfortunately there still are a lot of these left. I strongly advocate to rebase this PR onto the current master branch or better recreate it and cherry-pick and verify the desired commits. |
…terface Port planning_scene_interface to ROS2
* Minor fixup
Follows from #38 and particularly, after #38 (comment).